Pull request overview

This pull request improves Python prompt-template message serialization by using XML element serialization (instead of manual string concatenation) for Jinja2 and Handlebars message helpers, updates related samples to use serializer-backed helpers, and adds regression tests to ensure correct escaping and round-tripping of chat history with XML metacharacters.

Changes:

• Update Jinja2 and Handlebars message helpers to build message XML via ElementTree serialization to correctly escape XML metacharacters.
• Align Azure chat prompt-template samples to use message_to_prompt for serializer-backed message rendering.
• Add unit and end to end tests covering escaping and chat history round-trip behavior, including system message preservation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Automated Code Review

Reviewers: 3 | Confidence: 89%

✓ Security Reliability

This PR is a security improvement that replaces manual f-string XML construction with xml.etree.ElementTree for generating XML tags in both Handlebars and Jinja2 prompt template helpers. Previously, user content containing XML metacharacters (<, >, &, ") was injected verbatim into XML strings, which could corrupt the XML structure and cause incorrect message reconstruction via ChatHistory.from_rendered_prompt(). The fix uses Element/tostring for proper escaping, consistent with how the rest of the codebase (ChatMessageContent.to_element, ChatHistory.from_rendered_prompt) already handles XML. The use of xml.etree.ElementTree for serialization-only (not parsing) is safe and matches the existing nosec B405 pattern in 12+ other files; defusedxml is correctly used elsewhere for parsing. The sample files are updated to demonstrate the message_to_prompt helper. Test coverage is thorough, including round-trip tests that verify ChatHistory.from_rendered_prompt correctly recovers the original messages after escaping.

✓ Test Coverage

This PR switches the handlebars and jinja2 _message helpers from manual XML string concatenation to xml.etree.ElementTree, properly escaping XML metacharacters in message content and attribute values. The test additions are well-structured: they verify specific escape sequences (<, &, unescaped " in text), and critically verify round-trip fidelity via ChatHistory.from_rendered_prompt. Both template engines (handlebars and jinja2) have symmetric unit and e2e tests. Minor gaps: no test covers None/empty content in _message helpers (the jinja2 helper previously rendered None as the literal string "None" — now fixed but untested), and no test exercises attribute-value escaping in the handlebars _message block helper kwargs path. These are non-blocking.

✓ Design Approach

The PR correctly fixes an XML injection bug in both the Handlebars and Jinja2 _message helpers by switching from manual string concatenation to xml.etree.ElementTree.Element + tostring(), which properly escapes XML metacharacters in content. The approach is sound and the # nosec B405 annotations are appropriate (we are generating XML, not parsing untrusted input). The sample changes steering users toward message_to_prompt are a good idea since it uses to_prompt() / to_element() for full-fidelity serialization. However, the jinja2 _message(item) helper still only serializes item.content (the first TextContent text), silently dropping image items, function-call items, and any other non-text items from the message. Its sibling _message_to_prompt avoids this by delegating to item.to_prompt() / to_element(). Since _message(item) accepts a full ChatMessageContent, calers reasonably expect full serialization — yet the implementation silently produces an incomplete representation. The better approach for _message in jinja2 would be to call item.to_element() and use tostring() on the resulting element, exactly as to_prompt() does, rather than manually setting only the text field.

Suggestions

• The Jinja2 _message(item) helper sets message.text = item.content, which only captures the first TextContent's text and silently drops non-text items (images, function calls, etc.). Additionally, item.content could be a non-string type, and ElementTree.tostring behavior with non-strings is implementation-defined. The better approach is to delegate to item.to_element() and call tostring() on that—the same way _message_to_prompt works via item.to_prompt()—which would avoid data loss for multi-modal messages and eliminate the type-safety concern.
• Add a test for None/empty content in the Jinja2 _message helper—the old code rendered None as the literal string "None", and the new code correctly produces an empty element, but this fix is untested. A round-trip test with a ChatMessageContent that has no text items (e.g., a FunctionCallContent-only message) through {{ message(item) }} would lock this in.
• Consider adding a test for attribute values containing XML metacharacters in the Handlebars _message block helper (e.g., {{#message role=role custom_attr=value}}...{{/message}} where value contains < or "). The Element.set() call correctly escapes these, but there's no test verifying it.
• The Handlebars sample (azure_chat_gpt_api_handlebars.py) switched from {{#message role=role}}{{~content~}}{{/message}} to {{message_to_prompt}}, which changes the XML serialization format (from bare text to <text> child elements). While both formats round-trip correctly, this sample change is orthogonal to the XML-escaping fix and could be confusing to users who reference the old sample pattern.
• The Handlebars _message helper now sets message.text = str(options['fn'](this)), treating the entire block output as plain text. For users with allow_dangerously_set_content=True who previously relied on embedding raw XML sub-structure (e.g., image or function-call tags) directly in the block, this is a silent behavior change. It would be worth documenting that _message now always produces plain-text message content, and that message_to_prompt should be used for structured/multi-modal content.

Automated review by moonbox3's agents

Automated Code Review

Reviewers: 4 | Confidence: 87%

✓ Correctness

This PR fixes XML metacharacter escaping in both Handlebars and Jinja2 prompt template message helpers by switching from manual string concatenation to xml.etree.ElementTree for XML construction. The Handlebars _message helper now uses Element/tostring to properly escape content and attributes. The Jinja2 _message helper delegates to item.to_element()+tostring(), which changes its output format (content is now wrapped in child elements). Samples are updated to prefer message_to_prompt, and comprehensive tests verify round-trip correctness with XML metacharacters. The changes are logically correct and the tests properly validate the new escaping behavior.

✓ Security Reliability

This PR is a clear security improvement that fixes XML injection vulnerabilities in both the Handlebars and Jinja2 prompt template helpers. Previously, user-controlled content (message text, role values) was directly interpolated into XML strings via f-strings without escaping, allowing XML metacharacters like <, &, and " to break the XML structure or inject malicious content. The fix correctly switches to xml.etree.ElementTree.Element and tostring() for proper XML serialization with automatic escaping. The # nosec B405 suppression is appropriate since ElementTree is used only for XML generation, not parsing (where XXE risks exist). The new tests thoroughly verify escaping and round-trip fidelity. No blocking security or reliability issues found.

✓ Test Coverage

The PR properly escapes XML metacharacters in both the Handlebars and Jinja2 _message helpers by switching to xml.etree.ElementTree for XML generation. Test coverage is good overall: new unit tests and e2e tests verify escaping of <, &, and ", plus round-trip fidelity via ChatHistory.from_rendered_prompt. However, there are a few gaps: (1) no test covers the empty-content path added in the Handlebars helper (if content: message.text = content combined with short_empty_elements=False), (2) the Handlebars and Jinja2 _message helpers now produce structurally different XML (flat text vs <text> sub-elements via to_element()), and there is no test that validates both round-trip correctly for the same input, and (3) the > character is not tested even though it is an XML metacharacter.

✗ Design Approach

The PR correctly addresses XML injection by switching from f-string XML concatenation to xml.etree.ElementTree, which properly escapes metacharacters. However, it introduces a structural inconsistency between the two template-engine paths: the Jinja2 _message now delegates to item.to_element() (which wraps content in a <text> child element, producing <message role=".."><text>…</text></message>), while the Handlebars _message block helper still builds the element manually and sets message.text = content (producing the flat <message role="...">…</message> form). Because from_rendered_prompt must handle both formats and the serialization protocol is the only contract between the renderer and the parser, having two divergent XML shapes for the same logical object is a fragile, leaky design. The right fix is to unify both paths through item.to_element() / a shared serialisation helper, not to fix one engine while leaving the other inconsistent.

Automated review by chetantoshniwal's agents